Onboarding app redesign#1370
Conversation
Introduce a canonical dialog surface with structured header/body/footer slots and styling controls so feature pages can reuse a consistent modal foundation. Made-with: Cursor
Expose the new dialog component and related helper types from the package root so dashboard pages can import the shared modal API consistently. Made-with: Cursor
Add guidance for when and how to use DesignDialog so modal redesign work follows a single documented pattern across dashboard routes. Made-with: Cursor
Replace the hand-wired trigger history dialog chrome with the shared DesignDialog wrapper while preserving the existing summary header and trigger list behavior. Made-with: Cursor
Show confirmation, rich-header, tester, and parity examples so agents and developers can copy the shared modal patterns directly. Made-with: Cursor
Introduce a dedicated dialog playground entry with shape presets and generated snippets to make modal experimentation and reuse easier. Made-with: Cursor
Introduce a compact settings-row layout for the require-email-verification toggle, intended as the main onboarding control surface. Made-with: Cursor
Replace the legacy card/ActionDialog flow with the shared DesignDialog, DesignButton close affordances, and the settings-strip control component. Made-with: Cursor
Use singular/plural description text; move the total count next to the affected accounts label; shorten the list footer to “+ N more”. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new DesignDialog component and its package exports, documents and exposes it in the dashboard design guide and playground, refactors multiple dashboard pages to use Design system components (including dialog-based flows), and adds a small skills/doc + lockfile entry for a new "roids" skill. ChangesSkills & Documentation
DesignDialog Component & Dialog infra
Dashboard integration & page refactors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Redesigns multiple dashboard surfaces to the new design language, introducing a canonical DesignDialog wrapper and migrating several pages/components to the updated visual patterns.
Changes:
- Added
DesignDialogto@stackframe/dashboard-ui-componentsand documented it as the standard modal surface for the dashboard. - Refactored sign-up rules UI (rules list, trigger-history dialog, tester UI scaffolding) to use design-components and improved layout/atoms.
- Updated onboarding + auth methods pages/components to new card/setting patterns, plus added local UX helpers (menus, badges, alerts, etc.).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills-lock.json | Adds a skills lock entry (roids). |
| packages/dashboard-ui-components/src/index.ts | Re-exports new DesignDialog components/types. |
| packages/dashboard-ui-components/src/components/dialog.tsx | Introduces the canonical DesignDialog wrapper around dialog primitives. |
| apps/dashboard/src/components/ui/action-dialog.tsx | Adds outside-interaction dismissal controls + content class override. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx | Major redesign of the sign-up rules page; adopts design-components and new dialog usage in parts. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx | Migrates onboarding email-verification confirmation UX to DesignDialog and extracts setting UI. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsx | New compact “setting strip” component for email verification toggle. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsx | Adds DesignDialog demos and props documentation to design-language page. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx | Redesigns provider configuration dialog content + provider tile UI using design-components. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx | Redesigns auth methods settings layout (cards/rows/menus) and adds email verification toggle flow. |
| apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsx | Adds a playground section for testing DesignDialog variants/sizes/props. |
| apps/dashboard/DESIGN-GUIDE.md | Documents DesignDialog as the canonical dashboard modal surface and when to use it. |
| .agents/skills/roids/SKILL.md | Adds the roids skill documentation file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type MethodIcon = React.ElementType; | ||
|
|
There was a problem hiding this comment.
React is referenced as a type (React.ElementType) but the file does not import React (or ElementType) from react, which will cause a TypeScript compile error. Import the type from react (e.g., ElementType) or add a import type React from "react" and keep the alias.
| function NoteInline({ children }: { children: React.ReactNode }) { | ||
| return ( |
There was a problem hiding this comment.
React.ReactNode is used in the NoteInline prop type, but React is not imported, which will fail TypeScript compilation. Import React as a type (or import ReactNode directly from react) and update the annotation accordingly.
Greptile SummaryThis PR introduces a new
Confidence Score: 3/5The UI redesign is safe, but the onboarding confirmation dialog's Enable button will silently drop errors from the config update call. The Enable button in EnableEmailVerificationDialog calls await pendingChange.onConfirm() without any error boundary — a network failure during the config update is swallowed with no user feedback. The sign-up-rules trigger history dialog also bypasses the new DesignDialog abstraction by hand-copying its glassmorphic classes. Together with the hardcoded DOM ID in OnboardingEmailVerificationSetting, these warrant fixing before the onboarding flow ships. onboarding/page-client.tsx (async button handler), onboarding/onboarding-email-verification-setting.tsx (hardcoded switch ID), sign-up-rules/page-client.tsx (raw Dialog primitives) Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant SW as Toggle Switch
participant PC as PageClient
participant API as Admin API
participant DLG as EnableEmailVerificationDialog
U->>SW: Toggle ON
SW->>PC: onCheckedChange(true)
PC->>PC: runAsynchronouslyWithAlert(handleEmailVerificationChange)
PC->>API: previewAffectedUsersByOnboardingChange
API-->>PC: totalAffectedCount, affectedUsers
alt "totalAffectedCount > 0"
PC->>DLG: "open=true"
U->>DLG: Click Enable
DLG->>API: updateConfig
note over DLG,API: No runAsynchronouslyWithAlert - errors silently dropped
API-->>DLG: success
DLG->>PC: setPendingChange(null)
else No affected users
PC->>API: updateConfig direct
API-->>PC: success
end
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx:66-74
**Async `onClick` without `runAsynchronouslyWithAlert`**
The "Enable" button's `onClick` is async but not wrapped in `runAsynchronouslyWithAlert`. If `pendingChange.onConfirm()` throws (for example, a network error during the config update), the error is silently swallowed — the dialog stays open, the toggle appears stuck, and the user gets no feedback. `runAsynchronouslyWithAlert` should be used here to surface errors via the standard alert mechanism.
### Issue 2 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:478-481
**Hand-wired `DialogContent` duplicates the glassmorphic surface that `DesignDialog` centralizes**
This `DialogContent` manually copies the exact same blur/ring/shadow classes that `DesignDialog` was just introduced to provide. The design guide (§4.14) explicitly says not to hand-wire `Dialog + DialogContent + DialogHeader + DialogBody + DialogFooter` when building a full-page modal — the new `DesignDialog` component is the canonical surface for exactly this pattern.
```suggestion
<DialogContent
className="max-w-2xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]"
overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }}
>
{/* TODO: migrate to DesignDialog with size="2xl", icon={PulseIcon}, headerContent=... */}
```
### Issue 3 of 3
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsx:9-11
**Hardcoded `SWITCH_ID` causes duplicate DOM IDs if the component is rendered more than once**
Using a module-level constant for the `id`/`htmlFor` pair means every instance shares the same DOM ID. `MethodToggleRow` in the adjacent auth-methods page solves this correctly with `useId()`. If this component is rendered in more than one place, the `<label htmlFor>` will stop pointing to the right switch, breaking keyboard and screen-reader accessibility.
```suggestion
export type OnboardingEmailVerificationSettingProps = {
```
Reviews (1): Last reviewed commit: "Merge branch 'dev' into onboarding-app-r..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx (1)
416-439:⚠️ Potential issue | 🟠 MajorRemove the
returnfromfinally.Line 437 can suppress errors from the
try/catchpath and is already flagged by Biome’snoUnsafeFinally.Proposed fix
} finally { - if (nextRequestId !== latestRequestIdRef.current) return; - if (reset) setIsInitialLoading(false); - else setIsLoadingMore(false); + if (nextRequestId === latestRequestIdRef.current) { + if (reset) setIsInitialLoading(false); + else setIsLoadingMore(false); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx around lines 416 - 439, The finally block currently uses "if (nextRequestId !== latestRequestIdRef.current) return;" which can suppress errors from the try/catch; remove the return and instead guard only the side-effects: replace the early-return in the finally with a conditional that only skips the state-updating calls (use "if (nextRequestId === latestRequestIdRef.current) { if (reset) setIsInitialLoading(false); else setIsLoadingMore(false); }" or equivalent), keeping the same check used earlier (nextRequestId vs latestRequestIdRef.current) so you no longer return from finally and errors are not swallowed; update the finally in the function that contains latestRequestIdRef, nextRequestId, setIsInitialLoading, and setIsLoadingMore accordingly.
🧹 Nitpick comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx (1)
140-146: Encode provider IDs when building URLs.These URL/path strings interpolate
providerIddirectly. PreferencodeURIComponent()for consistency with the repo URL-construction rule.Suggested cleanup
function RedirectInline({ providerId }: { providerId: string }) { + const encodedProviderId = encodeURIComponent(providerId); return ( @@ - <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${providerId}`}</InlineCode> + <InlineCode>{`${getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL')}/api/v1/auth/oauth/callback/${encodedProviderId}`}</InlineCode> @@ function DocsTextLink({ providerId }: { providerId: string }) { + const docsProviderSlug = providerId === "x" ? "x-twitter" : providerId; return ( <Link - href={`https://docs.stack-auth.com/docs/concepts/auth-providers/${providerId === "x" ? "x-twitter" : providerId}`} + href={`https://docs.stack-auth.com/docs/concepts/auth-providers/${encodeURIComponent(docsProviderSlug)}`}As per coding guidelines,
Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs, for consistency even if it's not strictly necessary.Also applies to: 246-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx around lines 140 - 146, The RedirectInline component builds a callback URL by interpolating providerId directly; update this to encode the providerId (e.g., via encodeURIComponent or the repo's urlString helper) so paths are safe — modify the string construction inside RedirectInline (where getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL') and InlineCode are used) to encode providerId before concatenation, and apply the same change to the other occurrences noted around lines 246-250.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx (1)
602-606: Avoidanyfor the config update payload.This can be typed precisely without bypassing the type system.
Proposed fix
- const configUpdate: Record<string, any> = { 'auth.allowSignUp': newAllowSignUp }; + const configUpdate: Record<string, boolean | OAuthAccountMergeStrategy> = { + 'auth.allowSignUp': newAllowSignUp, + };As per coding guidelines,
Try to avoid the any type. Whenever you need to use any, leave a comment explaining why you're using it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx around lines 602 - 606, The configUpdate is typed as Record<string, any>; replace this with a precise type (e.g., an interface or existing ConfigUpdate/Partial<Config> type) that reflects the known keys and value types: 'auth.allowSignUp' should be boolean and 'auth.oauth.accountMergeStrategy' should be the specific enum/string type used by your auth config (use the same type as localMergeStrategy). Update the declaration of configUpdate (and any helper types) to use that concrete type and pass it to updateConfig({ adminApp: stackAdminApp, configUpdate, pushable: true }) so you avoid any and keep type-safety for config updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/roids/SKILL.md:
- Around line 14-16: Add an explicit production boundary to the implementation
instructions: state clearly that the TryRoids hosted runtime
(https://tryroids.com/roid-tool.js) and any wiring described in SKILL.txt are
intended only for sandboxed previews and must never be included in production
dashboard or app code; update the text near the "How it works" section to say
"preview-only / not for production." Also reconcile the loading contradiction by
clarifying that "wire" refers to using the inline markup pattern and data
attributes (e.g., data-roid-*) per SKILL.txt as documentation, and that agents
should not inject or rely on a <script src="https://tryroids.com/roid-tool.js">
in production — if needed for previews only reference the URL in docs or load it
in a sandboxed preview runner, not in app code.
In `@apps/dashboard/DESIGN-GUIDE.md`:
- Line 67: Update the stale section reference that currently points to "§22" to
the correct new section "§4.14" in the sentence that instructs using
DesignDialog (the line mentioning "DesignDialog" and "§22"); replace only the
section number so the sentence reads that DesignDialog is the canonical dialog
surface — see §4.14.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/design-language/page-client.tsx:
- Around line 1237-1245: Update the props table to match the real API: change
the "trigger" prop type to React.ReactElement to match DesignDialogProps, and
rename/document the close helper as DesignDialogClose (the exported re-export
used in examples) so the docs reflect the actual symbols; keep existing
descriptions but ensure the type cell for "trigger" shows "React.ReactElement"
and any reference to the close helper uses the identifier DesignDialogClose.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx:
- Around line 66-72: The Enable button handler currently calls
pendingChange.onConfirm() directly making the button remain clickable and
swallowing errors; update the DesignButton onClick to set a local loading state
(e.g., isConfirming) while awaiting the operation and use
runAsynchronouslyWithAlert (or runAsynchronously) to execute
pendingChange.onConfirm() so errors are surfaced to the alert helper and the UI
is disabled during the call; specifically, reference the DesignButton,
pendingChange, and its onConfirm method to wrap the call with
runAsynchronouslyWithAlert(...) and toggle the loading boolean so the button
shows a loading/disabled state and prevents double submits.
In `@packages/dashboard-ui-components/src/components/dialog.tsx`:
- Around line 60-78: The props type allows creating an unnamed modal; update the
API to enforce an accessible name by either making title required on
DesignDialogProps or adding a runtime guard in the DesignDialog component that
throws or logs an error when neither the standard title prop nor a custom
accessible header is provided (i.e., when title is falsy and customHeader does
not include a DialogTitle accessible element). Reference the DesignDialogProps
type (title, customHeader) and the DesignDialog component render path to
implement the change so the dialog always has a programmatic accessible name.
---
Outside diff comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:
- Around line 416-439: The finally block currently uses "if (nextRequestId !==
latestRequestIdRef.current) return;" which can suppress errors from the
try/catch; remove the return and instead guard only the side-effects: replace
the early-return in the finally with a conditional that only skips the
state-updating calls (use "if (nextRequestId === latestRequestIdRef.current) {
if (reset) setIsInitialLoading(false); else setIsLoadingMore(false); }" or
equivalent), keeping the same check used earlier (nextRequestId vs
latestRequestIdRef.current) so you no longer return from finally and errors are
not swallowed; update the finally in the function that contains
latestRequestIdRef, nextRequestId, setIsInitialLoading, and setIsLoadingMore
accordingly.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsx:
- Around line 602-606: The configUpdate is typed as Record<string, any>; replace
this with a precise type (e.g., an interface or existing
ConfigUpdate/Partial<Config> type) that reflects the known keys and value types:
'auth.allowSignUp' should be boolean and 'auth.oauth.accountMergeStrategy'
should be the specific enum/string type used by your auth config (use the same
type as localMergeStrategy). Update the declaration of configUpdate (and any
helper types) to use that concrete type and pass it to updateConfig({ adminApp:
stackAdminApp, configUpdate, pushable: true }) so you avoid any and keep
type-safety for config updates.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/auth-methods/providers.tsx:
- Around line 140-146: The RedirectInline component builds a callback URL by
interpolating providerId directly; update this to encode the providerId (e.g.,
via encodeURIComponent or the repo's urlString helper) so paths are safe —
modify the string construction inside RedirectInline (where
getPublicEnvVar('NEXT_PUBLIC_STACK_API_URL') and InlineCode are used) to encode
providerId before concatenation, and apply the same change to the other
occurrences noted around lines 246-250.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a77a4007-1f1d-4de3-86c2-7d436b0964ee
📒 Files selected for processing (13)
.agents/skills/roids/SKILL.mdapps/dashboard/DESIGN-GUIDE.mdapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/auth-methods/providers.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsxapps/dashboard/src/components/ui/action-dialog.tsxpackages/dashboard-ui-components/src/components/dialog.tsxpackages/dashboard-ui-components/src/index.tsskills-lock.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx:
- Around line 435-437: The finally block currently contains a premature return
when checking nextRequestId vs latestRequestIdRef which can mask exceptions;
remove the return statement and instead guard the state updates by conditionally
executing them only if nextRequestId === latestRequestIdRef.current, i.e.,
replace the `if (nextRequestId !== latestRequestIdRef.current) return;` with `if
(nextRequestId === latestRequestIdRef.current) { if (reset)
setIsInitialLoading(false); else setIsLoadingMore(false); }` so you preserve the
original behavior without using return in the finally block (referencing
nextRequestId, latestRequestIdRef, reset, setIsInitialLoading,
setIsLoadingMore).
- Around line 667-675: The dropdown handlers currently cast string values to
narrower union types; update ActionDropdown's onValueChange (which calls
state.setActionType) and the similar handler at the other location (around line
934 where DesignMenu is used for 'allow' | 'reject') to remove the unsafe casts
and instead perform explicit type-guard checks against the expected union values
(e.g., check membership in ACTION_DROPDOWN_OPTIONS keys or test value ===
'allow' || value === 'reject') before calling the setters; follow the same
pattern used elsewhere in this file (the type-narrowing guards used around lines
1148–1150 and 1222–1224) so you only call state.setActionType or the
allow/reject setter when the value passes the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4814e1cf-7de0-4a5a-8b0e-f4c477af8490
📒 Files selected for processing (4)
apps/dashboard/DESIGN-GUIDE.mdapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsx
- apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsx
|
@greptile-ai review |
| <DesignButton | ||
| size="sm" | ||
| onClick={async () => { | ||
| if (pendingChange == null) return; | ||
| await pendingChange.onConfirm(); | ||
| }} | ||
| > | ||
| <span>Enable</span> | ||
| </DesignButton> |
There was a problem hiding this comment.
Async
onClick without runAsynchronouslyWithAlert
The "Enable" button's onClick is async but not wrapped in runAsynchronouslyWithAlert. If pendingChange.onConfirm() throws (for example, a network error during the config update), the error is silently swallowed — the dialog stays open, the toggle appears stuck, and the user gets no feedback. runAsynchronouslyWithAlert should be used here to surface errors via the standard alert mechanism.
Rule Used: Use runAsynchronouslyWithAlert from `@stackframe... (source)
Learned From
stack-auth/stack-auth#943
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/page-client.tsx
Line: 66-74
Comment:
**Async `onClick` without `runAsynchronouslyWithAlert`**
The "Enable" button's `onClick` is async but not wrapped in `runAsynchronouslyWithAlert`. If `pendingChange.onConfirm()` throws (for example, a network error during the config update), the error is silently swallowed — the dialog stays open, the toggle appears stuck, and the user gets no feedback. `runAsynchronouslyWithAlert` should be used here to surface errors via the standard alert mechanism.
**Rule Used:** Use `runAsynchronouslyWithAlert` from `@stackframe... ([source](https://app.greptile.com/review/custom-context?memory=5e671275-7493-402a-93a8-969537ec4d63))
**Learned From**
[stack-auth/stack-auth#943](https://github.com/stack-auth/stack-auth/pull/943)
How can I resolve this? If you propose a fix, please make it concise.| <DialogContent | ||
| className="max-w-2xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]" | ||
| overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }} | ||
| > |
There was a problem hiding this comment.
Hand-wired
DialogContent duplicates the glassmorphic surface that DesignDialog centralizes
This DialogContent manually copies the exact same blur/ring/shadow classes that DesignDialog was just introduced to provide. The design guide (§4.14) explicitly says not to hand-wire Dialog + DialogContent + DialogHeader + DialogBody + DialogFooter when building a full-page modal — the new DesignDialog component is the canonical surface for exactly this pattern.
| <DialogContent | |
| className="max-w-2xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]" | |
| overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }} | |
| > | |
| <DialogContent | |
| className="max-w-2xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]" | |
| overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }} | |
| > | |
| {/* TODO: migrate to DesignDialog with size="2xl", icon={PulseIcon}, headerContent=... */} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx
Line: 478-481
Comment:
**Hand-wired `DialogContent` duplicates the glassmorphic surface that `DesignDialog` centralizes**
This `DialogContent` manually copies the exact same blur/ring/shadow classes that `DesignDialog` was just introduced to provide. The design guide (§4.14) explicitly says not to hand-wire `Dialog + DialogContent + DialogHeader + DialogBody + DialogFooter` when building a full-page modal — the new `DesignDialog` component is the canonical surface for exactly this pattern.
```suggestion
<DialogContent
className="max-w-2xl gap-0 p-0 overflow-hidden border-0 sm:rounded-2xl bg-background/85 backdrop-blur-2xl ring-1 ring-foreground/[0.06] shadow-[0_24px_48px_-12px_rgba(0,0,0,0.25),0_4px_24px_-8px_rgba(0,0,0,0.12)] dark:bg-background/80 dark:ring-white/[0.06]"
overlayProps={{ className: "bg-black/50 backdrop-blur-sm" }}
>
{/* TODO: migrate to DesignDialog with size="2xl", icon={PulseIcon}, headerContent=... */}
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const SWITCH_ID = "onboarding-email-verification"; | ||
|
|
||
| export type OnboardingEmailVerificationSettingProps = { |
There was a problem hiding this comment.
Hardcoded
SWITCH_ID causes duplicate DOM IDs if the component is rendered more than once
Using a module-level constant for the id/htmlFor pair means every instance shares the same DOM ID. MethodToggleRow in the adjacent auth-methods page solves this correctly with useId(). If this component is rendered in more than one place, the <label htmlFor> will stop pointing to the right switch, breaking keyboard and screen-reader accessibility.
| const SWITCH_ID = "onboarding-email-verification"; | |
| export type OnboardingEmailVerificationSettingProps = { | |
| export type OnboardingEmailVerificationSettingProps = { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/onboarding/onboarding-email-verification-setting.tsx
Line: 9-11
Comment:
**Hardcoded `SWITCH_ID` causes duplicate DOM IDs if the component is rendered more than once**
Using a module-level constant for the `id`/`htmlFor` pair means every instance shares the same DOM ID. `MethodToggleRow` in the adjacent auth-methods page solves this correctly with `useId()`. If this component is rendered in more than one place, the `<label htmlFor>` will stop pointing to the right switch, breaking keyboard and screen-reader accessibility.
```suggestion
export type OnboardingEmailVerificationSettingProps = {
```
How can I resolve this? If you propose a fix, please make it concise.
Summary by CodeRabbit
New Features
Improvements
Documentation
Documentation